Conversation
install-php-extensions
Outdated
| fi | ||
| ;; | ||
| pdo_snowflake) | ||
| if test $PHP_MAJMIN_VERSION -lt 801; then |
There was a problem hiding this comment.
I'd avoid this check: we don't do that for all the other extensions
install-php-extensions
Outdated
| printf 'pdo_snowflake requires PHP 8.1+\n' >&2 | ||
| exit 1 | ||
| fi | ||
| if ! command -v cmake >/dev/null 2>&1; then |
There was a problem hiding this comment.
I'd avoid this check: we don't do that for all the other extensions, and the cmake is installed thanks to the code in the buildRequiredPackageLists function.
install-php-extensions
Outdated
| installRemoteModule_tmp1="$(cmake --version | head -1 | grep -oE '[0-9]+\.[0-9]+' | head -1)" | ||
| installRemoteModule_tmp2="$(echo "$installRemoteModule_tmp1" | cut -d. -f1)" | ||
| installRemoteModule_tmp3="$(echo "$installRemoteModule_tmp1" | cut -d. -f2)" | ||
| if test "$installRemoteModule_tmp2" -lt 3 || { test "$installRemoteModule_tmp2" -eq 3 && test "$installRemoteModule_tmp3" -lt 17; }; then |
There was a problem hiding this comment.
I'd avoid this check: we don't do that for all the other extensions
install-php-extensions
Outdated
| printf 'cmake >= 3.17 required (detected: %s)\n' "$installRemoteModule_tmp1" >&2 | ||
| exit 1 | ||
| fi | ||
| if ! command -v gcc >/dev/null 2>&1; then |
There was a problem hiding this comment.
I'd avoid this check: we don't do that for all the other extensions, and gcc is installed thanks to the code in the buildRequiredPackageLists function.
install-php-extensions
Outdated
| exit 1 | ||
| fi | ||
| fi | ||
| installRemoteModule_tmp="$(getPackageSource "https://github.com/snowflakedb/pdo_snowflake/archive/refs/tags/v${installRemoteModule_version}.tar.gz")" |
install-php-extensions
Outdated
| exit 1 | ||
| fi | ||
| cd "$installRemoteModule_tmp" | ||
| export PHP_HOME="$(php-config --prefix)" |
There was a problem hiding this comment.
Is the PHP_HOME variable required by build_pdo_snowflake.sh? If so, I'd simply write
PHP_HOME="$(php-config --prefix)" ./scripts/build_pdo_snowflake.sh
install-php-extensions
Outdated
| rm -rf "$installRemoteModule_tmp" | ||
| exit 1 | ||
| fi | ||
| if ! test -f modules/pdo_snowflake.so; then |
There was a problem hiding this comment.
I'd avoid this check: the following cp will fail (and the script will abort) if that .so file doesn't exist
yinheli
force-pushed
the
feature/add-snowflake-pdo-driver
branch
from
ef2fe5f to
33e4ca6
Compare
|
Could you check the errors reported by CI? PS: no need to force-push changes (it makes review harder): when I'll merge this PR I'll squash the commits. |
yinheli
force-pushed
the
feature/add-snowflake-pdo-driver
branch
from
33e4ca6 to
22b5000
Compare
yinheli
force-pushed
the
feature/add-snowflake-pdo-driver
branch
from
22b5000 to
a0c094f
Compare
|
Thanks! |
2 participants
Summary
This PR adds support for the Snowflake PDO driver (
pdo_snowflake) extension, enabling PHP applications to connect to Snowflake databases using PDO.Limitations
Alpine Linux Not Supported
Snowflake's PDO driver currently does not support Alpine Linux (musl libc). This is a known upstream limitation tracked in snowflakedb/pdo_snowflake#319.
Users requiring Alpine compatibility should use Debian-based images instead:
php:8.3-alpinephp:8.3-cliorphp:8.3-fpmPHP 8.5 Not Supported
PHP 8.5 introduces PDO API changes that pdo_snowflake has not yet adapted to. Support will be added when upstream releases a compatible version.